Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

demux: add --autocreate-playlist #14555

Merged
merged 8 commits into from
Aug 10, 2024
Merged

Conversation

kasper93
Copy link
Contributor

Not tested

Copy link

github-actions bot commented Jul 15, 2024

Download the artifacts for this pull request:

Windows
macOS

@Dudemanguy
Copy link
Member

Dudemanguy commented Jul 16, 2024

Would it be worth adding a property to distinguish between a playlist that is created with this option? This PR currently will populate the playlist-path property with whatever you first opened mpv with which might be unexpected (e.g. local single file instead of a directory, playlist file, etc.)

player/loadfile.c Outdated Show resolved Hide resolved
@kasper93
Copy link
Contributor Author

Would it be worth adding a property to distinguish between a playlist that is created with this option? This PR currently will populate the playlist-path property with whatever you first opened mpv with which might be unexpected (e.g. local single file instead of a directory, playlist file, etc.)

I agree, but in the same time I find it quite telling that the playlist was created with a original file being the local file.

@kasper93
Copy link
Contributor Author

added filtering and docs

@kasper93 kasper93 marked this pull request as ready for review July 16, 2024 15:29
demux/demux.c Outdated Show resolved Hide resolved
@guidocella
Copy link
Contributor

--autocreate-playlist breaks external files like subtitles.

@DanOscarsson
Copy link
Contributor

While I have never wanted to give a file and have mpv generate a list of files in same directory, there is one thing I want that I think is missing:
I want to give mpv a directory and request mpv to play all audio files in that directory and sub directories.
While I today can give a directory for mpv to play all files in, I cannot request it to only play audio files.
Unless I am mistaken that is not solved here but ought to be easy to add to the code.

@kasper93
Copy link
Contributor Author

Unless I am mistaken that is not solved here but ought to be easy to add to the code.

I don't want to mix this changes, this is completely different. For example it would likely be a different option, because you want different filtering for autocreated playlist and different for directory opened directly.

@kasper93 kasper93 force-pushed the playlist branch 5 times, most recently from db2373d to e3d60ec Compare July 16, 2024 23:00
@kasper93
Copy link
Contributor Author

I want to give mpv a directory and request mpv to play all audio files in that directory and sub directories.

Added --directory-playlist-exts, since there were no other complains to work on.

DOCS/man/options.rst Outdated Show resolved Hide resolved
@DanOscarsson
Copy link
Contributor

Maybe combine normal directory playlists and autocreated?
Something like:

--playlist-video-exts=<ext-list>
--playlist-audio-exts=<ext-list>
--playlist-image-exts=<ext-list>

--autocreate-playlist=<no|any|filter|same>
--playlist-filter-exts=<no|video|audio|image>

So you can define the exts for audio/video/images if default is not wanted.
If autocreate-playlist is used it will be created from director of given file. Setting filter requests the defined filter exts to be used.
playlist-filter-exts is used for directory and autocreate. no means no filter is used, or you specify which ext set you want to filter on.
Maybe this option could allow more then one ext set to be specified so you could request for audio and video but no images.

This way the options get shorter names and works both for autocreated and for directory created.

@kasper93
Copy link
Contributor Author

@DanOscarsson: updated.

@kasper93 kasper93 force-pushed the playlist branch 3 times, most recently from cddea0d to 9ff5144 Compare July 17, 2024 19:14
@guidocella
Copy link
Contributor

--directory-filter-type=foo --directory-mode=lazy doesn't add subdirectories. Also these options could just be called --directory-video-exts and so on.

@kasper93
Copy link
Contributor Author

--directory-filter-type=foo --directory-mode=lazy doesn't add subdirectories. Also these options could just be called --directory-video-exts and so on.

Thanks guido, should be better now.

@na-na-hi
Copy link
Contributor

This PR doesn't have feature parity with autoload.lua yet, notably ignore patterns. Until this PR has 100% feature parity I don't think autoload.lua should be removed.

Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. I don't have any further things.

@kasper93
Copy link
Contributor Author

kasper93 commented Aug 10, 2024

I will merge this later. Honestly I don't have energy to look at this again, let users test it and fix remaining issues as they arrive.

@kasper93 kasper93 merged commit 4e50da3 into mpv-player:master Aug 10, 2024
25 checks passed
@kasper93 kasper93 deleted the playlist branch August 10, 2024 21:27
@verygoodlee
Copy link
Contributor

verygoodlee commented Aug 11, 2024

it breaks RESUMING PLAYBACK

autocreate-playlist=same
save-position-on-quit=yes
watch-later-options=start,speed,volume

values are saved correctly, but the start value is ineffective, always resume playback from the beginning.
image

@dyphire
Copy link
Contributor

dyphire commented Aug 11, 2024

There is also a problem that when opening a single file it creates the playlist normally with --autocreate-playlist=same, but when opening another file in another path it doesn't automatically create the playlist again, which doesn't match the expected behavior.

@kasper93
Copy link
Contributor Author

which doesn't match the expected behavior.

Says who? If there is a playlist present already, there will NOT be another one created or otherwise no new items will be appended based on autocreate logic.

@llyyr
Copy link
Contributor

llyyr commented Aug 11, 2024

values are saved correctly, but the start value is ineffective, always resume playback from the beginning.

https://github.com/mpv-player/mpv/blob/master/player/loadfile.c#L1682-L1684

We delete the watch_later config if we have a playlist

@dyphire
Copy link
Contributor

dyphire commented Aug 11, 2024

which doesn't match the expected behavior.

Says who?

This is not quite the same behavior as the autoload.lua script, which creates a new playlist based on the directory of another path when opening a file in another path with loadfile path command.

If there is a playlist present already, there will NOT be another one created or otherwise no new items will be appended based on autocreate logic.

The current behavior does not create a new playlist based on the new path with loadfile path command, it empties the current playlist and loads only one file, which is not wanted.

@kasper93
Copy link
Contributor Author

it empties the current playlist and loads only one file

Should be fixed by #14661. Test the build from the PR.

@dyphire
Copy link
Contributor

dyphire commented Aug 11, 2024

it empties the current playlist and loads only one file

Should be fixed by #14661. Test the build from the PR.

Yes, it fixes.

@candrapersada
Copy link

is there an option to automatically add new videos to the playlist if there are new videos in the folder?

@kasper93
Copy link
Contributor Author

is there an option to automatically add new videos to the playlist if there are new videos in the folder?

No.

@candrapersada
Copy link

candrapersada commented Sep 24, 2024

It's still better to use autoload.lua to detect newly added videos from downloads.

@kasper93
Copy link
Contributor Author

It's still better to use autoload.lua to detect newly added videos from downloads.

I don't think autoload.lua watches directory for new files. It may load them when the next file is loaded, but it is just an side-effect of how the script work.

@candrapersada
Copy link

candrapersada commented Sep 24, 2024

It may load them when the next file is loaded, but it is just an side-effect of how the script work.

Will there be a feature similar to load them when the next file is loaded?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.